Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.64%. Comparing base (b694901) to head (e25b787).
Report is 2 commits behind head on main.

@@            Coverage Diff            @@
##               main    #8955   +/-   ##
=========================================
  Coverage     67.64%   67.64%           
  Complexity     1168     1168           
=========================================
  Files           244      244           
  Lines          7783     7783           
  Branches        867      867           
=========================================
  Hits           5265     5265           
  Misses         2161     2161           
  Partials        357      357           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

I don't understand all of the code but wonder why we need both the CompositeBazelModuleRegistryService and the MultiBazelModuleRegistryService.

Is this due to the differences between Bazel <7.2 and >=7.2 lock files? Would it be possible to have those differences dealt with before MultiBazelModuleRegistryService is instantiated and thus have it cover both cases? Or am I completely misunderstanding the point?

I don't understand all of the code but wonder why we need both the CompositeBazelModuleRegistryService and the MultiBazelModuleRegistryService.

Is this due to the differences between Bazel <7.2 and >=7.2 lock files? Would it be possible to have those differences dealt with before MultiBazelModuleRegistryService is instantiated and thus have it cover both cases? Or am I completely misunderstanding the point?

You are mostly right; the different implementations deal with differences in the lock file formats. However, they apply different logic; therefore, I think they cannot be merged that easily:

• MultiBazelModuleRegistryService is used to handle multiple registries defined in a Bazel < 7.2 lock file. It is rather dump and simply tries the registries one after another until it gets a success result.
• CompositeBazelModuleRegistryService has more information available from a Bazel >= 7.2 lock file which lists the packages together with the URLs of registries from where they have been downloaded. Thus it can determine the registry to delegate to and does not have to use a trial-and-error approach.

There is a TODO comment in CompositeBazelModuleRegistryService to check for a package in other registries as well if it cannot be found in the one from which it was retrieved originally. This could maybe be implemented using a combination of the approaches that are currently there. But I would rather address this in a later refactoring step.

I have to admit that the names of the registry service implementations are probably not ideal. So, proposals for better names would be welcome.

* `CompositeBazelModuleRegistryService` has more information available from a Bazel >= 7.2 lock file which lists the packages together with the URLs of registries from where they have been downloaded. Thus it can determine the registry to delegate to and does not have to use a trial-and-error approach.

Thanks for the explanation! That makes sense.

I have to admit that the names of the registry service implementations are probably not ideal. So, proposals for better names would be welcome.

Yeah, I also thought the names are confusingly generic and similar, but putting the Bazel version number in them seems worse and I couldn't come up with anything else either.